RDKC-16495: Enhance DCM Agent to support log upload and backup functionality in RDK‑C - #162
Open
rdkcteam wants to merge 3 commits into
Open
RDKC-16495: Enhance DCM Agent to support log upload and backup functionality in RDK‑C#162rdkcteam wants to merge 3 commits into
rdkcteam wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the DCM Agent / uploadstblogs pipeline to support RDK-C (camera, sysvinit) platforms while keeping STB/broadband behavior unchanged by default, and documents/tests the new gating behavior.
Changes:
- Add RDK-C compatibility paths: guard extended rdk-logger init and retry the DCM→Telemetry reload-config event publish to avoid sysvinit start-order races.
- Add optional scheduled log staging into
DCM_LOG_PATHgated byDCM_SCHEDULED_LOG_COLLECT(default off for STB/broadband parity). - Update docs and add unit + functional tests covering the new scheduled log collection gate.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| uploadstblogs/unittest/context_manager_gtest.cpp | Adds unit tests verifying DCM_SCHEDULED_LOG_COLLECT toggles collect_scheduled_logs as intended. |
| uploadstblogs/src/uploadlogsnow.c | Exposes copy_files_to_dcm_path() for reuse by the scheduled DCM strategy and retains UploadLogsNow workflow. |
| uploadstblogs/src/strategies.c | Adds gated RDK-C scheduled log staging step in dcm_setup() using copy_files_to_dcm_path(). |
| uploadstblogs/src/context_manager.c | Adds logger fallback when RDK_LOGGER_EXT is absent and reads DCM_SCHEDULED_LOG_COLLECT into runtime context. |
| uploadstblogs/include/uploadstblogs_types.h | Adds collect_scheduled_logs to RuntimeContext. |
| uploadstblogs/include/uploadlogsnow.h | Documents and exports copy_files_to_dcm_path() for reuse outside UploadLogsNow. |
| uploadstblogs/docs/requirements/uploadSTBLogs_requirements.md | Documents the optional RDK-C scheduled staging requirement. |
| uploadstblogs/docs/hld/uploadSTBLogs_HLD.md | Adds an HLD section describing the RDK-C scheduled log collection addition. |
| test/functional-tests/tests/test_uploadstblogs_upload_strategies.py | Adds L2 functional tests validating staging behavior when the device property is enabled/disabled. |
| Readme.txt | Adds pointer to README.md for full documentation. |
| README.md | Documents RDK-C/sysvinit notes, build flags, logger fallback, and scheduled log collection behavior. |
| docs/Logupload_Behavior/README.md | Documents the RDK-C scheduled log collection behavior in the behavior guide. |
| dcm.c | Adds bounded retry loop for reload-config RBUS event publish to complete the DCM–Telemetry handshake on sysvinit. |
Comment on lines
+53
to
+57
| * Copies each regular file from @p src_path into @p dest_path, excluding the | ||
| * `dcm/`, `PreviousLogs/`, and `PreviousLogs_backup/` sub-directories (so it never | ||
| * recurses into its own staging/backup dirs). Used by the on-demand UploadLogsNow | ||
| * workflow and by the scheduled DCM strategy on platforms that opt into current-log | ||
| * collection (RDK-C, DCM_SCHEDULED_LOG_COLLECT). |
| retryCount++; | ||
| DCMInfo("Reload event not delivered yet, retry %d/%d\n", | ||
| retryCount, DCM_RELOAD_EVENT_MAX_RETRY); | ||
| sleep(1); |
| * @note Returns 0 for empty directories (this is a valid success case, not an error) | ||
| */ | ||
| static int copy_files_to_dcm_path(const char* src_path, const char* dest_path) | ||
| int copy_files_to_dcm_path(const char* src_path, const char* dest_path) |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (4)
uploadstblogs/src/uploadlogsnow.c:103
- Making copy_files_to_dcm_path() non-static introduces a duplicate symbol with the unit test stub (uploadstblogs/unittest/uploadlogsnow_gtest.cpp:121) because uploadlogsnow_gtest links ../src/uploadlogsnow.c (uploadstblogs/unittest/Makefile.am:145). This will fail to link the gtest binary.
int copy_files_to_dcm_path(const char* src_path, const char* dest_path)
uploadstblogs/src/uploadlogsnow.c:56
- The error log for fopen() failure prints STATUS_FILE where callers would expect the OS error (errno). This makes diagnosing permission/path issues difficult.
if (!fp) {
RDK_LOG(RDK_LOG_ERROR, LOG_UPLOADSTB,
"[%s:%d] Failed to open status file: %s\n",
__FUNCTION__, __LINE__, STATUS_FILE);
return -1;
uploadstblogs/src/uploadlogsnow.c:165
- copy_files_to_dcm_path() calls copy_file() on every directory entry without checking whether it's a regular file. Depending on what copyFiles() supports, this can cause noisy warnings, unexpectedly large copies (if directories are copied), and symlink traversal risks. Consider lstat() + S_ISREG() filtering before copying.
// Use file operations utility for copy
if (copy_file(src_file, dest_file)) {
copied_count++;
RDK_LOG(RDK_LOG_DEBUG, LOG_UPLOADSTB,
"[%s:%d] Copied: %s\n", __FUNCTION__, __LINE__, entry->d_name);
uploadstblogs/src/context_manager.c:416
- load_environment() only sets collect_scheduled_logs when the property is "true"; it never explicitly resets it to false. If load_environment() is called on a non-zeroed RuntimeContext (e.g., config reload), a previously-true value will persist even when the device property is removed/false.
memset(buffer, 0, sizeof(buffer));
if (getDevicePropertyData("DCM_SCHEDULED_LOG_COLLECT", buffer, sizeof(buffer)) == UTILS_SUCCESS) {
if (strcasecmp(buffer, "true") == 0) {
ctx->collect_scheduled_logs = true;
RDK_LOG(RDK_LOG_INFO, LOG_UPLOADSTB, "[%s:%d] Scheduled DCM log collection enabled\n", __FUNCTION__, __LINE__);
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enables dcmd on RDK-C camera (sysvinit) platforms and closes the daily scheduled-upload gap, with no impact to STB/broadband. Docs- and test-only for the final commit; the two functional commits are RDK-C-guarded / default-off.
• Logger compatibility — guard the extended rdklogger init behind RDK_LOGGER_EXT; fall back to standard rdk_logger_init(debug.ini) for RDK-C's older rdk-logger 2.4.0. Fleet (which defines the flag) is unaffected.
• DCM–T2 handshake race on sysvinit — retry the reload-config event publish up to DCM_RELOAD_EVENT_MAX_RETRY (30 × 1 s) so the handshake completes regardless of start order. On systemd it succeeds first try → identical behavior.
• Scheduled log collection — new DCM_SCHEDULED_LOG_COLLECT device property stages the current /opt/logs tree into DCM_LOG_PATH before the daily DCM upload (RDK-C cameras have no external batcher). Default off ⇒ STB/broadband batch-drain unchanged.
• Docs + tests — RDK-C/sysvinit platform notes, -DRDK_LOGGER_EXT/-DRDKC flags, and DCM_SCHEDULED_LOG_COLLECT behavior across README.md, Logupload_Behavior/, uploadstblogs/docs/{hld,requirements}; L1 (context_manager_gtest.cpp) and L2 (test_uploadstblogs_upload_strategies.py) coverage for the staging flag.